allow options to be overriden by david-driscoll · Pull Request #2262 · RocketSurgeonsGuild/Testing

david-driscoll · 2026-07-04T17:47:23Z

No description provided.

codacy-production · 2026-07-04T17:49:34Z

Not up to standards ⛔

🔴 Issues 1 critical

Alerts:
⚠ 1 issue (≤ 0 issues of at least minor severity)

Results:
1 new issue

Category Results

ErrorProne 1 critical

View in Codacy

🟢 Metrics 0 complexity · 0 duplication

Metric Results

Complexity 0

Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

_{TIP This summary will be updated as you push new changes.}

github-actions · 2026-07-04T17:49:56Z

Test Results

0 tests ±0 0 ✅ ±0 0s ⏱️ ±0s
0 suites ±0 0 💤 ±0
0 files ±0 0 ❌ ±0

Results for commit 5012eea. ± Comparison against base commit 373a597.

codacy-production

Pull Request Overview

The PR aims to allow overriding options and marked locations, but the implementation is currently inconsistent. While metadata is now overwritten using SetItem, the actual source content in AddMarkup is still being appended, which will lead to duplicate file definitions in the resulting compilation (e.g., CS0101).

Additionally, the code is currently not up to standards due to a high-severity risk in OptionsProvider.cs where a potential null reference could occur during property initialization. There is also no test coverage provided for the overriding logic or the refactored provider, which is critical for verifying that the new behavior works as intended without regressing existing functionality.

About this PR

The PR description is missing, providing no context on the motivation for these changes.

Test suggestions

Invoke AddOption twice for the same path and key; verify the resulting context contains the second value.
Invoke AddGlobalOption twice for the same key; verify the second value overwrites the first.
Invoke AddMarkup twice with the same name; verify the _markedLocations dictionary contains the location/trigger from the second call.
Verify that OptionsProvider correctly merges global options into file options without overwriting existing file-specific keys.

Prompt proposal for missing tests

Consider implementing these tests if applicable:
1. Invoke `AddOption` twice for the same path and key; verify the resulting context contains the second value.
2. Invoke `AddGlobalOption` twice for the same key; verify the second value overwrites the first.
3. Invoke `AddMarkup` twice with the same name; verify the `_markedLocations` dictionary contains the location/trigger from the second call.
4. Verify that `OptionsProvider` correctly merges global options into file options without overwriting existing file-specific keys.

_{TIP Improve review quality by adding custom instructions}
_{TIP How was this review? Give us feedback}

codacy-production · 2026-07-04T17:50:49Z

-        {
-            return _properties.TryGetValue(key, out value);
-        }
+        public override bool TryGetValue(string key, [NotNullWhen(true)] out string? value) => _properties.TryGetValue(key, out value);


_{🔴 HIGH RISK}

The _properties field is initialized from the primary constructor parameter without a null check. If properties is null, any call to TryGetValue will result in a NullReferenceException. Consider initializing with a null check: private readonly ImmutableDictionary<string, string> _properties = properties ?? ImmutableDictionary<string, string>.Empty;.

See Issue in Codacy

codacy-production · 2026-07-04T17:50:49Z

        {
-            _markedLocations = _markedLocations.Add(name, new(source.Location, source.Trigger)),
+            _markedLocations = _markedLocations.SetItem(name, new(source.Location, source.Trigger)),
            _sources = _sources.Add(new(SourceText.From(source.Code, Encoding.UTF8)) { Name = name, }),


_{🟡 MEDIUM RISK}

While _markedLocations was updated to SetItem to allow overriding, _sources remains additive. To fully support overriding markup, the existing source with the same name must be replaced in the _sources collection; otherwise, the compilation will contain multiple files with the same name, leading to 'Duplicate type defined' compiler errors (e.g., CS0101).

codacy-production · 2026-07-04T17:50:49Z

        if (!options.TryGetValue(tree.FilePath, out var value)) return GlobalOptions;
        foreach (var v in globalOptions.Where(v => !value.ContainsKey(v.Key)))
        {
-            value = value.Add(v.Key, v.Value);
+            value = value.SetItem(v.Key, v.Value);
        }

        return new OptionsObject(value);


_{⚪ LOW RISK}

Suggestion: This merge logic can be simplified and made more efficient by using SetItems to apply local overrides to the global options.

Suggested change

if (!options.TryGetValue(tree.FilePath, out var value)) return GlobalOptions;

foreach (var v in globalOptions.Where(v => !value.ContainsKey(v.Key)))

{

value = value.Add(v.Key, v.Value);

value = value.SetItem(v.Key, v.Value);

}

return new OptionsObject(value);

if (!options.TryGetValue(tree.FilePath, out var value)) return GlobalOptions;

return new OptionsObject(globalOptions.SetItems(value));

codacy-production · 2026-07-04T17:50:49Z

        var rootOptions = _options;
-        if (rootOptions.TryGetValue(path, out var fileOptions))
+        if (_options.TryGetValue(path, out var fileOptions))
        {
-            fileOptions = fileOptions.Add(key, value);
+            fileOptions = fileOptions.SetItem(key, value);
            rootOptions = rootOptions.SetItem(path, fileOptions);
        }
        else
        {
-            rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.Add(key, value));
+            rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.SetItem(key, value));
        }

        return this with { _options = rootOptions, };


_{⚪ LOW RISK}

Suggestion: This logic can be simplified to a single expression using GetValueOrDefault.

Suggested change

var rootOptions = _options;

if (rootOptions.TryGetValue(path, out var fileOptions))

if (_options.TryGetValue(path, out var fileOptions))

{

fileOptions = fileOptions.Add(key, value);

fileOptions = fileOptions.SetItem(key, value);

rootOptions = rootOptions.SetItem(path, fileOptions);

}

else

{

rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.Add(key, value));

rootOptions = rootOptions.SetItem(path, ImmutableDictionary<string, string>.Empty.SetItem(key, value));

}

return this with { _options = rootOptions, };

var fileOptions = _options.GetValueOrDefault(path, ImmutableDictionary<string, string>.Empty).SetItem(key, value);

return this with { _options = _options.SetItem(path, fileOptions) };

allow options to be overriden

5012eea

david-driscoll enabled auto-merge July 4, 2026 17:47

github-actions Bot added this to the v10.0.6 milestone Jul 4, 2026

david-driscoll added this pull request to the merge queue Jul 4, 2026

codacy-production Bot reviewed Jul 4, 2026

View reviewed changes

Merged via the queue into main with commit dcf7db1 Jul 4, 2026
7 of 8 checks passed

david-driscoll deleted the feature/fix-add branch July 4, 2026 17:52

github-actions Bot added the ✨ mysterious We forgot to label this label Jul 4, 2026

github-actions Bot modified the milestones: v10.0.6, v10.0.7 Jul 4, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

allow options to be overriden#2262

allow options to be overriden#2262
david-driscoll merged 1 commit into
mainfrom
feature/fix-add

david-driscoll commented Jul 4, 2026

Uh oh!

codacy-production Bot commented Jul 4, 2026

Uh oh!

github-actions Bot commented Jul 4, 2026

Uh oh!

codacy-production Bot left a comment

Uh oh!

codacy-production Bot Jul 4, 2026

Uh oh!

codacy-production Bot Jul 4, 2026

Uh oh!

codacy-production Bot Jul 4, 2026

Uh oh!

codacy-production Bot Jul 4, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

david-driscoll commented Jul 4, 2026

Uh oh!

codacy-production Bot commented Jul 4, 2026

Not up to standards ⛔

Uh oh!

github-actions Bot commented Jul 4, 2026

Test Results

Uh oh!

codacy-production Bot left a comment

Choose a reason for hiding this comment

Pull Request Overview

About this PR

Test suggestions

Uh oh!

codacy-production Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

codacy-production Bot Jul 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant